test+feat: close rich Markdown preview residuals - #209
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR centralizes accepted rich Markdown detection and validation. Import preview handoff now supports accepted v3 links, emphasis, inline code, and strikethrough. Tests cover valid, malformed, oversized, and concealed constructs. Status and ADR documentation reflect the expanded compatibility behavior. ChangesRich Markdown handoff
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ImportPreview
participant MarkdownCompiler
participant AcceptedConstructDetector
participant LeasedFileScan
ImportPreview->>MarkdownCompiler: compile import preview
MarkdownCompiler-->>ImportPreview: return refusal and unsupported construct
ImportPreview->>AcceptedConstructDetector: validate accepted inline construct
AcceptedConstructDetector-->>ImportPreview: return acceptance result
ImportPreview->>LeasedFileScan: hand off eligible content
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 283495547d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| elif candidate[0] == fence[0] and len(candidate) >= len(fence): | ||
| if not fence_body_has_content: | ||
| return False | ||
| fence = None |
There was a problem hiding this comment.
Require a bare closing fence before handing off
When an otherwise accepted document contains a fenced block whose would-be closer has a suffix, such as ```oops, this condition treats that line as the closing fence because it checks only the marker character and length. The v3 parser's _is_closing_fence requires the marker to be followed only by whitespace, so the worker instead refuses the document as CODE_BLOCK; the preview therefore returns an actionable scan handoff for malformed syntax that ADR-0093 says must remain generically unavailable. Validate the complete closing-fence line using the same rule as the v3 parser.
AGENTS.md reference: AGENTS.md:L22-L24
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@engine/supply/markdown.py`:
- Around line 715-750: Update _has_closed_rich_html_block and its call from
contains_only_accepted_rich_markdown_inline so the closing-tag search is limited
to the source from the current opening line onward, preventing an earlier
same-tag closing element from satisfying a later unclosed block. Preserve the
existing first-line call behavior for the other caller.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: d5211060-2bc7-4251-922a-2058dca710bb
📒 Files selected for processing (7)
STATUS.mdadapters/http/ui_api.pydocs/decisions/0093-activate-leased-rich-markdown-and-revision-link-graph.mdengine/supply/__init__.pyengine/supply/markdown.pytests/integration/test_import_preview_requires_confirm.pytests/unit/test_markdown_compiler.py
|
E3 substantive findings are fixed on commits
Verification with repository-pinned Node 22.12.0:
One earlier full integration attempt had a timing-sensitive scheduler ordering failure ( |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0bee808c0c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if _RICH_SETEXT_PATTERN.fullmatch(line) is not None: | ||
| if ( | ||
| previous_line is not None | ||
| and previous_line.strip() | ||
| and unsupported_rich_markdown_inline(previous_line.strip()) is not None | ||
| ): | ||
| return False | ||
| list_open = False | ||
| previous_line = line | ||
| continue | ||
| if _THEMATIC_BREAK_PATTERN.fullmatch(line) is not None: | ||
| list_open = False | ||
| previous_line = line | ||
| continue |
There was a problem hiding this comment.
Reject indented separators while a list is open
stometa, when an accepted rich construct is followed by a list continuation such as - item\n ordinary\n ---, this branch treats the indented separator as a standalone setext/thematic marker and returns a scan handoff. The v3 parser instead processes it as an indented list continuation and refuses it as FRONTMATTER_OR_RULE, so malformed input bypasses the ADR-0093 requirement that compilation refusals remain generically unavailable. Check list-continuation context before handling setext and thematic separators.
AGENTS.md reference: AGENTS.md:L22-L24
Useful? React with 👍 / 👎.
| html_block_end = index | ||
| while ( | ||
| html_block_end + 1 < len(lines) | ||
| and lines[html_block_end + 1].strip() | ||
| ): | ||
| html_block_end += 1 |
There was a problem hiding this comment.
Preserve setext precedence before grouping HTML blocks
stometa, for input such as [ok](note.md)\n\n<div>ok</div>\n===\n<div>bad, this loop groups all three consecutive lines into a closed HTML block and the preview returns a scan handoff. The v3 parser first interprets <div>ok</div> plus === as a setext heading, then refuses the unclosed <div>bad as HTML; consequently a malformed document receives the actionable handoff that ADR-0093 reserves for v3-accepted documents. Mirror the compiler's setext lookahead before extending the HTML block.
AGENTS.md reference: AGENTS.md:L22-L24
Useful? React with 👍 / 👎.
Summary
return "[" in sourcemutation is rejectedCloses #207
Verification
make lint— PASS (All checks passed!)make typecheck— PASS (mypy:Success: no issues found in 494 source files; TypeScript SDK/action/bot checks passed)make test— PASS (2480 passed in 68.94s)17 passed, 69 deselected)28 passed in 15.29s)make db-reset— PASS on the generated worktree-specific PostgreSQL 17 + pgvector harnessmake check— all targets completed; the PTY lost its closing buffered chunk, so results were independently checked from captured component output and fresh artifacts:726/730 tests collected (4 deselected)170/170registered tests passed, no collection errors,m0SecurityDecision: pass, bound to commit283495547d728257508c14fa530f33d39f250f1agit diff --check— PASSMutation evidence
Temporarily replacing the link predicate with
return "[" in sourcecaused the named negative test to report 5 failures and 1 pass. The production predicate was restored; the committed test passes all six cases.The worktree-specific database harness was stopped after verification; its disposable data volume was preserved.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation